src/server/internal-routes.ts (3)

16-18: Match '/_agentuity' with or without trailing slash and strip query.

Current check misses '/_agentuity' and paths with query. Parse pathname first.

-	isInternalRoute(url: string): boolean {
-		return url.startsWith('/_agentuity/');
-	}
+	isInternalRoute(url: string): boolean {
+		const pathname = url.startsWith('http')
+			? new URL(url).pathname
+			: url.split('?')[0];
+		return pathname === '/_agentuity' || pathname.startsWith('/_agentuity/');
+	}

---

31-32: Return structured 404 with content-type.

Helps clients and avoids text bodies.

-		return new Response('Internal route not found', { status: 404 });
+		return new Response(JSON.stringify({ error: 'not_found' }), {
+			status: 404,
+			headers: { 'content-type': 'application/json' },
+		});

---

9-11: Keep the logger for future, but store it for consistency.

Small polish; no behavior change.

-export class InternalRoutesHandler {
-	constructor(_logger: Logger) {
-		// Logger parameter kept for future use
-	}
+export class InternalRoutesHandler {
+	constructor(private readonly logger: Logger) {}

src/apis/evaljobscheduler.ts (3)

30-35: Use slice instead of deprecated substr.

Avoid deprecated String.prototype.substr.

- this.instanceId = `EvalJobScheduler-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`;
+ this.instanceId = `EvalJobScheduler-${Date.now()}-${Math.random()
+  .toString(36)
+  .slice(2, 11)}`;

---

40-60: getInstance doesn’t need to be async.

No awaits present; returning Promise adds overhead for callers.

- public static async getInstance(): Promise<EvalJobScheduler> {
+ public static getInstance(): EvalJobScheduler {
   // ...unchanged...
-  return globalThis.__evalJobSchedulerInstance;
+  return globalThis.__evalJobSchedulerInstance!;
 }

Note: adjust call sites accordingly.

---

99-104: Consider TTL or max-capacity to prevent unbounded growth.

Add eviction (e.g., max jobs per session, time-based cleanup).

src/apis/eval.ts (6)

248-289: Validate and normalize score/pass results before emitting.

• Clamp score to [0,1].
• Keep comment and behavior aligned.

- score: (
-  val: number,
+ score: (
+  val: number,
   meta?: { reasoning?: string; [key: string]: unknown }
 ) => {
+  const score = Math.max(0, Math.min(1, val));
   createEvalRunRequest = {
     projectId,
     sessionId,
     spanId,
     result: {
       success: true,
-      score: val,
+      score,
       metadata: {
         reason: meta?.reasoning || '',
         ...meta,
       },
     },
     evalId: metadata.id,
     promptHash,
   };
 },

---

225-233: Guard missing projectId.

If AGENTUITY_CLOUD_PROJECT_ID is unset, fail fast or warn loudly.

- const projectId = process.env.AGENTUITY_CLOUD_PROJECT_ID || '';
+ const projectId = process.env.AGENTUITY_CLOUD_PROJECT_ID || '';
+ if (!projectId) {
+   internal.warn('AGENTUITY_CLOUD_PROJECT_ID is not set; eval runs may not persist.');
+ }

---

334-335: Use slice instead of deprecated substr in ID generation.

- id: `local-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`,
+ id: `local-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`,

---

410-417: Make metadata regex resilient to type annotations.

Handles export const metadata: Type = { ... }.

- const metadataRegex = /export\s+const\s+metadata\s*=\s*\{/;
+ const metadataRegex =
+  /export\s+const\s+metadata(?:\s*:\s*[^=]+)?\s*=\s*\{/;

---

445-463: String-munging JSON is brittle.

Quotes in values, nested objects, or keys with hyphens will break. Prefer importing the module and reading metadata or use a tiny parser for the export.

Would you like me to replace this with a safe static analysis using a parser (e.g., acorn) or switch to dynamic import for metadata only?

---

117-152: Sequential dynamic imports may be slow for many evals.

Optional: parallelize with Promise.allSettled on filtered files, then pick the first match.

Also applies to: 170-205

src/router/router.ts (1)

428-438: Re-check pending header semantics.

Since you now await waitUntilAll, hasPending() should usually be false; consider removing the header or gating wait by config.

Would you like me to add a config flag (e.g., awaitWaitUntilAll: boolean) to preserve previous non-blocking behavior?

src/server/bun.ts (2)

65-67: Safer global leakage for internal logger

Avoid a plain string property on globalThis. Prefer a Symbol or a unique namespace object to reduce collision risk.

Apply this diff:

-	(globalThis as any).__agentuityInternalLogger = logger;
+	const kInternalLogger = Symbol.for('@agentuity/internalLogger');
+	(globalThis as any)[kInternalLogger] = logger;

---

178-191: Internal routes: add CORS, generate runId, and guard body

• Add '*' CORS header for parity with Node handler.
• Pass a generated runId (helps downstream logging/tracing).
• Guard req.body (can be null in Bun).

Apply this diff:

-				// Handle internal routes first
-				if (url.pathname.startsWith('/_agentuity/')) {
-					const internalResponse = await internalRoutes.handleInternalRoute({
-						method,
-						url: url.pathname,
-						headers: req.headers.toJSON(),
-						body: req.body as unknown as ReadableStream<ReadableDataType>,
-						request: getRequestFromHeaders(req.headers.toJSON(), ''),
-						setTimeout: (_val: number) => void 0,
-					});
-					if (internalResponse) {
-						return internalResponse;
-					}
-				}
+				// Handle internal routes first
+				if (url.pathname.startsWith('/_agentuity/')) {
+					const runId =
+						globalThis.crypto?.randomUUID?.() ??
+						Date.now().toString(36);
+					const internalResponse = await internalRoutes.handleInternalRoute({
+						method,
+						url: url.pathname,
+						headers: req.headers.toJSON(),
+						body:
+							(req.body ??
+								undefined) as unknown as ReadableStream<ReadableDataType>,
+						request: getRequestFromHeaders(req.headers.toJSON(), runId),
+						setTimeout: (_val: number) => void 0,
+					});
+					if (internalResponse) {
+						const headers = new Headers(internalResponse.headers);
+						if (!headers.has('Access-Control-Allow-Origin')) {
+							headers.set('Access-Control-Allow-Origin', '*');
+						}
+						return new Response(internalResponse.body, {
+							status: internalResponse.status,
+							headers,
+						});
+					}
+				}

src/index.ts (1)

1-1: ESM path consistency for bundler/NodeNext

New imports/exports should include “.js” to match existing style (e.g., promptMetadata, patchportal) and avoid runtime resolution issues under node16/nodenext.

Apply:

-export * from './apis/eval';
+export * from './apis/eval.js';
@@
-export { hash, hashSync } from './utils/hash';
+export { hash, hashSync } from './utils/hash.js';
@@
-import EvalAPI from './apis/eval';
-import EvalJobScheduler from './apis/evaljobscheduler';
+import EvalAPI from './apis/eval.js';
+import EvalJobScheduler from './apis/evaljobscheduler.js';

As per coding guidelines.

Also applies to: 5-5, 16-17

src/utils/promptMetadata.ts (1)

75-83: Unify hashing and fix typos (comiledPrompts) in concat path

• Use hashSync instead of direct crypto for consistency.
• Rename comiledPrompts → compiledPrompts.
• Drop the unused crypto import afterwards.

Apply:

-export async function processPromptMetadataConcat(
-	comiledPrompts: string[]
-): Promise<string> {
-	const compiledPromptsString = comiledPrompts.join('');
+export async function processPromptMetadataConcat(
+	compiledPrompts: string[]
+): Promise<string> {
+	const compiledPromptsString = compiledPrompts.join('');
@@
-		comiledPrompts.map(async (compiledPrompt) => {
+		compiledPrompts.map(async (compiledPrompt) => {
@@
-	const newCompiledHash = crypto
-		.createHash('sha256')
-		.update(comiledPrompts.join(''))
-		.digest('hex');
+	const newCompiledHash = hashSync(compiledPrompts.join(''));

Additionally remove the top-level crypto import if now unused:

-import crypto from 'crypto';

As per coding guidelines.

Also applies to: 97-101

src/router/context.ts (1)

134-139: Validate eval slugs using loaded metadata

You load evalMetadataMap but never use it. Validate slugs to avoid futile runEval calls and clearer logs.

Apply:

@@
-			const evalMetadataMap = await evalAPI.loadEvalMetadataMap();
+			const evalMetadataMap = await evalAPI.loadEvalMetadataMap();
 			logger.info(`📚 Loaded ${evalMetadataMap.size} eval mappings`);
@@
-			for (const evalSlug of promptMeta.evals) {
+			for (const evalSlug of promptMeta.evals) {
+				if (!evalMetadataMap.has(evalSlug)) {
+					logger.warn(
+						`⏭️  Unknown eval '${evalSlug}' — skipping (not in metadata map)`
+					);
+					continue;
+				}
 				try {

Also applies to: 205-206

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: jhaynie
PR: agentuity/sdk-js#179
File: src/router/context.ts:36-58
Timestamp: 2025-09-24T18:33:34.725Z
Learning: In the AgentContextWaitUntilHandler class in src/router/context.ts, the running counter should only be decremented in waitUntilAll, not when individual promises complete. The design intent is that running represents work that hasn't been fully processed as a batch, maintaining the collective nature of the wait-until pattern where a session isn't done until all deferred work is processed together.

Learnt from: CR
PR: agentuity/sdk-js#0
File: .cursor/rules/code-generation.mdc:0-0
Timestamp: 2025-10-03T22:20:06.957Z
Learning: Applies to src/apis/prompt/index.ts : When loading generated modules, guard with fs.existsSync and wrap require() in try/catch; default to safe content on failure

Learnt from: CR
PR: agentuity/sdk-js#0
File: .cursor/rules/code-generation.mdc:0-0
Timestamp: 2025-10-03T22:20:06.957Z
Learning: Applies to src/apis/prompt/index.ts : Use dynamic loading with absolute paths; check dist first then src, and provide fallbacks when generated content is missing

Learnt from: CR
PR: agentuity/sdk-js#0
File: .cursor/rules/code-generation.mdc:0-0
Timestamp: 2025-10-03T22:20:06.957Z
Learning: Applies to src/apis/prompt/index.ts : Use require() (CommonJS) to load generated modules at runtime rather than import(), ensuring bundler compatibility

Learnt from: CR
PR: agentuity/sdk-js#0
File: .cursor/rules/code-generation.mdc:0-0
Timestamp: 2025-10-03T22:20:06.957Z
Learning: Applies to src/apis/**/*.ts : Do not hardcode generated prompt content (e.g., copyWriter) in source files; load it dynamically

Learnt from: CR
PR: agentuity/sdk-js#0
File: .cursor/rules/code-generation.mdc:0-0
Timestamp: 2025-10-03T22:20:06.957Z
Learning: Applies to src/apis/prompt/**/*.ts : Generated content must import from agentuity/sdk rather than relative paths

Learnt from: CR
PR: agentuity/sdk-js#0
File: .cursor/rules/code-generation.mdc:0-0
Timestamp: 2025-10-03T22:20:06.957Z
Learning: Applies to src/apis/**/*.ts : Avoid relative require('./generated/_index.js'); resolve absolute paths from process.cwd() into node_modules for generated assets

src/apis/evaljobscheduler.ts (2)

119-149: Logging uses debug level appropriately.

The method logs job details at internal.debug level throughout, which is appropriate for detailed operational data. This addresses the concern from the previous review about info-level logging.

---

154-168: printState still dumps full jobs array with potential sensitive data.

The method continues to serialize complete job objects without redaction. Prompt metadata may contain sensitive inputs, outputs, or other PII.

Apply the previously suggested fix:

-	public printState(): void {
-		internal.debug('🔍 EvalJobScheduler.printState() called');
-		internal.debug('🔍 Instance ID:', this.instanceId);
-		internal.debug('🔍 EvalJobScheduler State:');
-		internal.debug('📊 Total jobs:', this.pendingJobs.size);
-		internal.debug('📋 Job IDs:', Array.from(this.pendingJobs.keys()));
-		internal.debug(
-			'📦 All jobs:',
-			JSON.stringify(Array.from(this.pendingJobs.values()), null, 2)
-		);
-		internal.debug(
-			'🔍 Global instance check:',
-			globalThis.__evalJobSchedulerInstance === this
-		);
-	}
+	public printState(): void {
+		internal.debug('🔍 EvalJobScheduler.printState() called');
+		internal.debug('🔍 Instance ID:', this.instanceId);
+		internal.debug('🔍 EvalJobScheduler State:');
+		internal.debug('📊 Total jobs:', this.pendingJobs.size);
+		internal.debug('📋 Job IDs:', Array.from(this.pendingJobs.keys()));
+		if (process.env.AGENTUITY_DEBUG_EVALS === '1') {
+			internal.debug(
+				'📦 All jobs (redacted):',
+				JSON.stringify(
+					Array.from(this.pendingJobs.values()).map(j => ({
+						spanId: j.spanId,
+						sessionId: j.sessionId,
+						promptMetadataCount: j.promptMetadata?.length ?? 0,
+						hasOutput: !!j.output,
+						createdAt: j.createdAt,
+					})),
+					null,
+					2
+				)
+			);
+		}
+		internal.debug(
+			'🔍 Global instance check:',
+			globalThis.__evalJobSchedulerInstance === this
+		);
+	}

src/router/context.ts (1)

82-86: Zero-promises branch still missing markSessionCompleted call.

The early-return path executes evals but never marks the session completed, leaving the session in an incomplete state from a tracking perspective. This inconsistency with the Promise.all branch (line 96) means metrics and downstream consumers won't see the session end.

Apply this fix:

 		if (this.promises.length === 0) {
 			internal.debug('No promises to wait for, executing evals directly');
+			await markSessionCompleted(sessionId, 0);
 			await this.executeEvalsForSession(logger, sessionId);
 			return;
 		}

src/apis/evaljobscheduler.ts (1)

40-60: Consider making getInstance() synchronous.

The method is declared async but performs no asynchronous operations. Unless async initialization is planned for the future, this could be simplified to a synchronous method returning EvalJobScheduler directly.

If desired, apply this diff:

-	public static async getInstance(): Promise<EvalJobScheduler> {
+	public static getInstance(): EvalJobScheduler {
 		internal.debug('🔍 EvalJobScheduler.getInstance() called');
 		internal.debug(
 			'🔍 globalThis.__evalJobSchedulerInstance exists:',
 			!!globalThis.__evalJobSchedulerInstance
 		);

 		if (!globalThis.__evalJobSchedulerInstance) {
 			globalThis.__evalJobSchedulerInstance = new EvalJobScheduler();
 			internal.debug(
 				'🆕 Created new EvalJobScheduler instance, ID:',
 				globalThis.__evalJobSchedulerInstance.instanceId
 			);
 		} else {
 			internal.debug(
 				'♻️ Returning existing EvalJobScheduler instance, ID:',
 				globalThis.__evalJobSchedulerInstance.instanceId
 			);
 		}
 		return globalThis.__evalJobSchedulerInstance;
 	}

src/router/context.ts (1)

112-174: Consider reusing EvalAPI instance or document stateless design.

A new EvalAPI instance is created for each session (line 138). If EvalAPI is stateless and lightweight, this is fine. Otherwise, consider reusing a single instance or caching the evalMetadataMap to avoid repeated file system scans.

Also, ensure cleanup failures are non-critical: the removeJob() loop (lines 167-169) doesn't check return values, which is acceptable if job removal is best-effort.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/apis/eval.ts (2)

29-33: Replace any with unknown in index signature.

This issue was already flagged in a previous review. The index signature should use unknown to align with strict TypeScript guidelines, and the biome-ignore comment should be removed.

---

317-331: Improve HTTP status validation to check 2xx range.

The current check if (!resp.status) only verifies that a status exists, not whether it indicates success. A proper HTTP status check should validate the 2xx range (200-299).

Apply this diff:

-if (!resp.status) {
-	internal.debug('Failed to update the database with the eval run', {
+if (!resp || typeof resp.status !== 'number' || resp.status < 200 || resp.status >= 300) {
+	internal.debug('Failed to update the database with the eval run', {
 		status: resp.status,
 		evalName,
 		evalId: r.evalId,
 		sessionId: r.sessionId,
 	});
 } else {
 	internal.debug('Eval run updated in the database', {
 		status: resp.status,
 		evalName,
 		evalId: r.evalId,
 		sessionId: r.sessionId,
 	});
 }

src/apis/eval.ts (1)

111-158: Consider extracting shared logic to reduce duplication.

Both loadEvalByName and loadEvalById share nearly identical file-scanning, filtering, and dynamic-import logic. Only the metadata matching criterion differs (slug vs. id). Extracting a private helper method would improve maintainability.

Example refactor:

private async findEvalByPredicate(
	predicate: (metadata: any) => boolean
): Promise<{
	evalFn: EvalFunction;
	metadata: { id: string; slug: string; name: string; description: string };
}> {
	const files = fs.readdirSync(this.evalsDir);
	const expectedExt = this.isBundled ? '.js' : '.ts';

	for (const file of files) {
		if (file === 'index.ts' || file === 'index.js') continue;
		if (!file.endsWith(expectedExt)) continue;

		const filePath = path.join(this.evalsDir, file);
		try {
			const fileUrl = pathToFileURL(filePath).href;
			const module = await import(fileUrl);

			if (module.metadata && predicate(module.metadata)) {
				return { evalFn: module.default, metadata: module.metadata };
			}
		} catch (error) {
			internal.debug(`Skipping file ${file} due to import error: ${error}`);
		}
	}
	throw new Error('No eval found matching predicate');
}

async loadEvalByName(evalName: string) {
	return this.findEvalByPredicate(m => m.slug === evalName);
}

async loadEvalById(evalId: string) {
	return this.findEvalByPredicate(m => m.id === evalId);
}

Also applies to: 164-211

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro